Add Synchronous Metric Instruments SDK #179
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #179 +/- ##
=======================================
Coverage 92.19% 92.19%
=======================================
Files 117 117
Lines 4038 4038
=======================================
Hits 3723 3723
Misses 315 315 |
ankit-bhargava
changed the title
reyang
reviewed
Jul 29, 2020
| {} | ||
|
|
||
| /** | ||
| * Returns a Bound Instrument associated with the specified labels. * Multiples requests |
reyang
reviewed
Jul 29, 2020
| // Utility function which converts maps to strings for better performance | ||
| inline std::string mapToString(const std::map<std::string,std::string> & conv){ | ||
| std::stringstream ss; | ||
| ss <<"{ "; |
There was a problem hiding this comment.
There are several places in this PR where the code is not well-formatted (e.g. spacing, indentation, etc.)
There was a problem hiding this comment.
Sorry about that, for some reason these weren't an issue in my IDE. I'll be sure to go over them carefully and take care of all the issues.
There was a problem hiding this comment.
went back over and tried to clean up most of these
Merged
reyang
approved these changes
Jul 30, 2020
reyang
added
the
pr:please-merge
|
@ankit-bhargava would you rebase and resolve the conflicts? |
ankit-bhargava
force-pushed
the
metrics-sdk-syncinstr
branch
from
cc7fb1f to
a332250
Compare
|
@reyang I had some issues rebasing so please let me know if anything looks off. Otherwise, this should be ready to go! |
ankit-bhargava
force-pushed
the
metrics-sdk-syncinstr
branch
from
921e53e to
42da878
Compare
ankit-bhargava
force-pushed
the
metrics-sdk-syncinstr
branch
from
02a72da to
4d90ee7
Compare
|
@reyang Recently rebased, think this can be merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements classes described in the Metric Instruments API (PR #161). These classes, unlike the no-op implementations included with the API, are capable of actually collecting telemetry data and sit at the beginning of the Metrics data pipeline as they capture data from instrumented applications. Instruments employ Aggregators (pending approval in PR #178 ) to combine the updates they receive into meaningful values. The PR will not compile without these files.
A test suite which validates each instrument’s ability to record values in single-threaded and concurrent scenarios is included. Functions necessary for the metrics pipeline as a whole such as reference counting are tested as well.
This PR is part of a series of PRs to add a full implementation of metrics API and SDK support.
Note: We templated instruments to support a wider array of scalar types. Templating prevents us from using out of line declarations as we normally would. As a result, we left all implementation code in the header file. If there are any suggestions to remedy this we will gladly make the change.